Skip to content

Comments

Manchester | 26-ITP-Jan | Liban Jama | Sprint 2 | Form-Controls#1082

Open
libanj0161 wants to merge 12 commits intoCodeYourFuture:mainfrom
libanj0161:tshirt-form
Open

Manchester | 26-ITP-Jan | Liban Jama | Sprint 2 | Form-Controls#1082
libanj0161 wants to merge 12 commits intoCodeYourFuture:mainfrom
libanj0161:tshirt-form

Conversation

@libanj0161
Copy link

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist
I created a form to collect customer data. The form consists of their name, email, t-shirt colour and size. I used only HTML. All the inputs have labels they are associated with. I scored 100 in the Lighthouse Accessibility score. Their name and email has to be a valid one and they can only pick one colour and size out of the options given.

Questions
N/A.

@netlify
Copy link

netlify bot commented Jan 27, 2026

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 3277e6e
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/699b2632fb882a0008a0365c
😎 Deploy Preview https://deploy-preview-1082--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@libanj0161 libanj0161 added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Onboarding The name of the module. labels Jan 27, 2026
<input id="Extra Extra Large"type="radio" name="size" value="XXL" />
<label for="Extra Extra Large">Extra Extra Large</label> <br>


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we submit the form? is there an element to use to submit the form ?

<br>

<p>Size</p>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done for the good work.
just a few more things to do
can we use a dropdown menu () for the sizes ? when should you use radio buttons and drop down menu ()
see https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/select

</p>
<p>
<label for="mail">Email</label>
<input type="email" id="mail" name="user_email" />
Copy link

@takanocap takanocap Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use a placeholder to guide the users?
can we check for a minlength too ?
is there a way to validate the inputs?
can we use an attribute for the validation?

</p>
<p>Colour</p>

<label>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an attribute to associate the label with the radio button ?

@takanocap takanocap added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 27, 2026
<form>
<p>
<label for="name">Name</label>
<input type="text" id="name" name="user_name" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use a minlength and other attribute to validate or make sure the input is required ?
can we use the placeholder to guide the users ?

@takanocap takanocap self-assigned this Jan 28, 2026
@libanj0161 libanj0161 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 2, 2026
@libanj0161 libanj0161 requested a review from takanocap February 4, 2026 18:01
@takanocap takanocap removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 5, 2026
<form>
<p>
<label for="name">Name</label>
<input type="text" id="name" name="user_name" placeholder="John Smith" required maxlength="18"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if someone enters only 1 character ? example "w" what is the minimum expected length for name input ? can we use minlength for validation here ?

Comment on lines 51 to 52
<input type="email" id="email" name="user_email" placeholder="johnsmith@hotmail.com" required maxlength="32"/>
</p>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when a user enters a@b as email? can we use minlength for validation ?


<p>Size</p>

<label for="size">Size</label>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we validate the form element to make sure users select a value?

<p>Colour</p>

<label for="red">
<input id="red" type="radio" name="colour" value="red" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when should you use a radio button vs select ? which is more appropriate in this scenario and why ? can you also validate this ?

@libanj0161 libanj0161 requested a review from takanocap February 6, 2026 11:29
@libanj0161 libanj0161 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 6, 2026
@cjyuan
Copy link
Contributor

cjyuan commented Feb 13, 2026

@libanj0161
It's considered a good practice to respond to every reviewer's question and comment.
Can you take a look at this "Addressing Reviewer Feedback" Guide
and take this opportunity to practice them?

I think you have addressed most but not all of the reviewer's comments.

Can you take look at this General Feedback to see if there is anything you can do to make your PR more robust and ready?

@libanj0161
Copy link
Author

Afternoon, I have made the necessary changes. I have added submit elemet, added a dropdown menu for sizes, added placeholders, minimum and maximum lengths, required. Thank you.

Copy link

@CameronDowner CameronDowner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a question in this comment you should try and answer: #1082 (comment)

Comment on lines 44 to 51
<select id="size" name="size" required>
<option value="xs">XS</option>
<option value="s">S</option>
<option value="m">M</option>
<option value="l">L</option>
<option value="xl">XL</option>
<option value="xxl">XXL</option>
</select>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small error that comes up when I validate this HTML. Can you check what's showing & try and fix it?

https://validator.w3.org/nu/#textarea

<label for="name">Name</label>
<input type="text" id="name" name="user_name"
placeholder="John Smith"
required minlength="2" maxlength="18">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want maxlength on name / email address? Could a name be longer than 18 characters, or an email longer than 32 characters?

@libanj0161
Copy link
Author

  • I removed max length from name and email fields.
  • I also wrapped the radio buttons in and changed

    Colour

    to Colour.
  • I wrapped the size dropdown in and added a placeholder option.

@libanj0161 libanj0161 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 21, 2026
@cjyuan
Copy link
Contributor

cjyuan commented Feb 22, 2026

@libanj0161

  • According to https://validator.w3.org/, there are errors in your code. Can you fix them? (If the validator shows errors or warnings, that means the HTML code is not prepared properly).
  • Can you improve the Lighthouse Accessibility score from 93 to 100?
  • The radio button inputs and dropdown list do not look right.
image
  • Some of the code are not properly indented. Consider using VSCode's "Format Document" feature to format the code for better readability and consistency.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 22, 2026
@libanj0161 libanj0161 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 22, 2026
@libanj0161
Copy link
Author

ive made all the changes you have put forward. Thank you.

Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Form is greatly improved. Just a few minor changes needed.

Comment on lines 83 to 85
<footer>
<h2>By LIBAN JAMA</h2>
</footer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Headings (<h1><h6>) are meant to label sections of content.

Use of <h2> would make "By LIBAN JAMA" sounds like a header of a major section to a screen reader and to search engines. <p> is probably more appropriate here.

<fieldset>
<legend>Select Size</legend>

<p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<p> would signal this is a paragraph or some text content. <div>, a generic block-level element, is more appropriate if you only need to create enough spacing to pass the Lighthouse Accessibility check.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 22, 2026
@libanj0161
Copy link
Author

ive done those changes, please check, thank you.

@cjyuan
Copy link
Contributor

cjyuan commented Feb 22, 2026

You only changed one <p> to <div>. Why not change all that are applicable?

And after you have made all the necessary changes, you may want to check if there are any syntax error that may be introduced by accident.

@libanj0161
Copy link
Author

I've changed the relevant p tags to div tags and have checked for syntax errors for which there are none.

@cjyuan
Copy link
Contributor

cjyuan commented Feb 22, 2026

Changes look good! Good job.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Feb 22, 2026
@takanocap takanocap removed their assignment Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Onboarding The name of the module. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants